Skip to content

fix(gcs): map cacheControl and customTime on the gRPC object path - #184

Open
exoego wants to merge 3 commits into
floci-io:mainfrom
exoego:fix/gcs-grpc-cache-control-custom-time
Open

fix(gcs): map cacheControl and customTime on the gRPC object path#184
exoego wants to merge 3 commits into
floci-io:mainfrom
exoego:fix/gcs-grpc-cache-control-custom-time

Conversation

@exoego

@exoego exoego commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

UpdateObject rejected the mask paths. Both are now mapped like their siblings.

A live GCS probe also set the rules: an unset custom_time or null customTime keeps the value, a decrease is rejected, and only contentEncoding is honoured as an upload query parameter.

Type of change

  • Bug fix (fix:)
  • New feature (feat:)
  • Breaking change (feat!: or fix!:)
  • Docs / chore

GCP Compatibility

  • gRPC GetObject / ListObjects returned an empty cache_control and no custom_time for objects written over REST, and UpdateObject rejected both mask paths. They now maps both fields.
  • An unset custom_time under the mask, or a JSON customTime: null, cleared the field. They are now kept.
  • A decrease of customTime was accepted. It is now rejected with INVALID_ARGUMENT / 400; the message is pinned in tests.
  • An empty cache_control under the mask stored "". It is now unset.
  • customTime was stored as sent. It is now rendered in UTC and rejected with 400 if unparsable values.
  • System metadata on the upload URL was applied. Only contentEncoding is honoured now.

Checklist

  • ./mvnw test passes locally
  • New or updated integration test added
  • Commit messages follow Conventional Commits

UpdateObject rejected the mask paths. Both are now mapped like their siblings.

A live GCS probe also set the rules: an unset custom_time or null customTime
keeps the value, a decrease is rejected, and only contentEncoding is honoured
as an upload query parameter.
@exoego
exoego marked this pull request as ready for review September 7, 2026 06:04
@greptile-apps

greptile-apps Bot commented Sep 7, 2026

Copy link
Copy Markdown

Greptile Summary

This PR aligns GCS object system metadata behavior across REST and gRPC paths.

  • Maps cacheControl and customTime through gRPC writes, updates, reads, and listings.
  • Normalizes and validates custom timestamps while preserving GCS no-clear and no-decrease semantics.
  • Restricts upload URL system metadata to contentEncoding.
  • Adds focused controller and integration coverage for metadata round-tripping and error behavior.
  • No shared-interface incompatibility was found in the related floci-az repository.
  • The required greptile_confidence label could not be added because GitHub CLI authentication is unavailable.

Confidence Score: 5/5

The PR appears safe to merge, with no outstanding correctness or repository-rule violations identified.

The earlier malformed-protobuf timestamp issue is fixed by explicit nanos and seconds validation, while the legacy-value concern was correctly withdrawn because the unvalidated persistence path was never released. The current changes consistently normalize and map both metadata fields and include automated coverage across REST and gRPC paths.

Important Files Changed

Filename Overview
src/main/java/io/floci/gcp/services/gcs/GcsCustomTime.java Centralizes custom-time normalization, protobuf conversion, bounds handling, and monotonicity validation.
src/main/java/io/floci/gcp/services/gcs/GcsGrpcMapper.java Maps cache control and custom time between stored metadata and gRPC objects and update masks.
src/main/java/io/floci/gcp/services/gcs/GcsService.java Applies normalized custom-time patch semantics and rejects decreases.
src/main/java/io/floci/gcp/services/gcs/GcsUploadController.java Limits upload query metadata to content encoding and validates custom time from JSON metadata.
src/test/java/io/floci/gcp/services/gcs/GcsGrpcControllerTest.java Covers gRPC metadata round-tripping, clearing behavior, monotonicity, and malformed timestamp handling.
src/test/java/io/floci/gcp/services/gcs/GcsSystemMetadataRestIntegrationTest.java Covers REST upload and patch behavior against the documented GCS semantics.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  REST[REST upload or patch] --> CT[GcsCustomTime validation]
  GRPC[gRPC write or update] --> CT
  CT --> META[GcsObjectMeta]
  META --> RESTREAD[REST object response]
  META --> MAPPER[GcsGrpcMapper]
  MAPPER --> GRPCREAD[gRPC GetObject or ListObjects]
Loading

Reviews (3): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile

Comment thread src/main/java/io/floci/gcp/services/gcs/GcsGrpcMapper.java
Comment thread src/main/java/io/floci/gcp/services/gcs/GcsGrpcMapper.java Outdated
@hectorvent hectorvent added bug Something isn't working storage Cloud Storage (GCS) labels Sep 10, 2026
@hectorvent

Copy link
Copy Markdown
Contributor

One thing, and it is not about your code: #180 merged since I looked at this and added to GcsGrpcControllerTest.java, so this now conflicts there. GcsService.java and GcsGrpcMapper.java still merge cleanly.

Blocking: a rebase, whenever you get a moment. That is our merge ordering, not anything you did.

Follow-up: worth recording, since it makes your case stronger than the PR text does — the upload-URL narrowing is in the spec, not only probe-confirmed. objects.insert lists contentEncoding as its only system-metadata parameter (storage v1 discovery); cacheControl, customTime, contentDisposition, contentLanguage and storageClass are absent, so honouring them there was the deviation you removed.

Greptile's legacy-timestamp point: your refutation holds, and I checked it rather than taking it. customTime became writable in #170, which is not an ancestor of 0.8.0.

Thank you for chasing the live behaviour rather than guessing — the cross-protocol test is exactly the shape these gaps needed.

…ntrol-custom-time

# Conflicts:
#	src/test/java/io/floci/gcp/services/gcs/GcsGrpcControllerTest.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working storage Cloud Storage (GCS)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants